-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
determine default branch #278
Conversation
@@ -110,7 +106,7 @@ export function getInputs(): IGitSourceSettings { | |||
core.debug(`recursive submodules = ${result.nestedSubmodules}`) | |||
|
|||
// Auth token | |||
result.authToken = core.getInput('token') | |||
result.authToken = core.getInput('token', {required: true}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this a breaking change? Does the action need a token if checking out a public repo?
If we're making this required, we should update the action.yml file with required: true
for this input as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Today it already needs a token but fails with a bad error #221
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally it doesnt need to be specified, because the default value works.
However we've seen cases where it's empty:
- Folks have set it to
''
- Due to fork PR a secret doesnt get mapped in
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's empty, can I still checkout from a public repo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No today it fails like this:
/usr/bin/git -c protocol.version=2 fetch --no-tags --prune --progress --no-recurse-submodules --depth=1 origin +559278e5d4074a5495dceca4d32edaa1acc01724:refs/remotes/origin/master
##[error]fatal: could not read Username for 'https://github.com': terminal prompts disabled
@@ -8,7 +8,7 @@ inputs: | |||
description: > | |||
The branch, tag or SHA to checkout. When checking out the repository that | |||
triggered a workflow, this defaults to the reference or SHA for that | |||
event. Otherwise, defaults to `master`. | |||
event. Otherwise, uses the default branch. | |||
token: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we update this to have required: true
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not required that the user enter a value. It has a default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
4ec7ea5
to
9d2f5b3
Compare
Thanks for implementing this! 🎉 |
return yield retryHelper.execute(() => __awaiter(this, void 0, void 0, function* () { | ||
core.info('Retrieving the default branch name'); | ||
const octokit = new github.GitHub(authToken); | ||
const response = yield octokit.repos.get({ owner, repo }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ericsciple We are using this action on our repository's .wiki
repo, and our respective action has been failing recently as the API returns "Not Found" for those. Could this be an oversight here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the heads up! And sorry for the disruption :(
I will look into the scenario shortly...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Fortunately was an easy fix on our end :)
Is it even possible to change the wiki default branch name? If not, maybe for those repositories the old hard-coded "logic" could be maintained for now.
Fixes #210
Fixes #221